Skip to content

fix(frontend): unify stop-token semantics across model schedulers - #865

Open
RicardoMin wants to merge 1 commit into
pegainfer-project:mainfrom
RicardoMin:fix/request-stop-token-policy
Open

fix(frontend): unify stop-token semantics across model schedulers#865
RicardoMin wants to merge 1 commit into
pegainfer-project:mainfrom
RicardoMin:fix/request-stop-token-policy

Conversation

@RicardoMin

@RicardoMin RicardoMin commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

This PR introduces a typed stop contract for stepped engines and implements it end-to-end for Qwen3, currently the only model using the stepped-engine path.

I discovered the issue while validating Qwen3 through the OpenAI-compatible completions API: explicitly configured stop_token_ids did not terminate generation when the matching token was produced.

Problem

The stepped frontend did not carry primary EOS and request-level stop_token_ids as independent stopping conditions. As a result, Qwen3 could not distinguish whether generation stopped because of EOS or because of an explicit request stop token.

The intended contract is:

ignore_eos Primary EOS stops Explicit stop_token_ids stop
false Yes Yes
true No Yes

ignore_eos should only control primary EOS handling. It should not disable explicit stop tokens supplied by the request.

Changes

  • Added typed StopPolicy and StopCause contracts for stepped engines.
  • Propagated primary EOS and explicit stop tokens independently from the stepped frontend into Qwen3.
  • Applied the contract to Qwen3 prefill, decode, unified, and speculative paths.
  • Preserved the triggering token, its real logprob, and completion-token accounting.
  • Truncated speculative output after the first stopping token.
  • Made a token stop take precedence when it coincides with max_tokens.
  • Reported explicit stop tokens through stop_reason, while primary EOS has no stop_reason.
  • Rejected non-zero min_tokens before scheduler submission because stepped engines do not support its required logits masking yet.

Scope

This PR changes the stepped frontend contract and its Qwen3 implementation only.

Qwen3.5, Kimi-K2, DeepSeek-V2-Lite, GLM5.2, and Gemma4 still use the legacy EngineHandle path, so their runtime behavior is unchanged. If this contract direction is accepted, those model paths can be migrated and validated separately.

Validation

Check Result
cargo fmt --all -- --check Passed
git diff --check Passed
Frontend library tests 68 passed, 0 failed
Qwen3 library tests 92 passed, 0 failed
Real Qwen3-0.6B eager HTTP cases Passed
Eager mixed-policy concurrency 12/12 passed
CUDA Graph mixed-policy concurrency 12/12 passed
CUDA Graph EOS comparison Passed
Legacy interface compilation for Qwen3.5, DeepSeek-V2-Lite, Kimi-K2, and Gemma4 Passed

The real HTTP coverage included explicit stop tokens with both values of ignore_eos, length termination, streaming, include_stop_str_in_output, and primary-EOS behavior.

For stop_token_ids = [17], both values of ignore_eos produced:

token_ids         = [220, 16, 11, 220, 17]
finish_reason     = stop
stop_reason       = 17
completion_tokens = 5

Primary EOS was independently verified:

Case Result
ignore_eos = false Stopped at token 151645; finish_reason = stop; stop_reason = null
ignore_eos = true Continued past token 151645 and later finished with finish_reason = length

Known upstream limitation

The repository currently pins vLLM revision 8e61b646e2d157f9b93451fa048f9c8530c8a67b.

In that revision, when the first generated token is itself a stopping token, the detokenizer may incorrectly flush prompt text into the completion. This is a separate upstream issue fixed by vLLM PR #47707, so this PR does not add a local workaround.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57270613ef

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread pegainfer-frontend/src/vllm/wire.rs
@xiaguan xiaguan self-assigned this Aug 17, 2026
@xiaguan

xiaguan commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

will review later

@xiaguan

xiaguan commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

The upstream limitation is already fixed by vLLM PR #47707 (merged at cc706b05). PegaInfer currently pins 8e61b646, which predates that fix.

I locally bumped the five vLLM Rust crates to cc706b05. pegainfer-frontend needed one compatibility update (EngineCoreOutput::ec_transfer_params: None), after which cargo check -p pegainfer-frontend --all-targets passed.

Could you please open a separate PR for the vLLM pin bump? Please keep the lockfile update narrowly scoped and include the zero-generated-token flush regression plus frontend compile/tests. With that bump in place, #865 can rely on the upstream fix instead of carrying a local detokenizer workaround.

@RicardoMin

Copy link
Copy Markdown
Contributor Author

Thanks for confirming. I’ll open a separate PR to bump the five vLLM Rust crates to cc706b05, keep the lockfile update narrowly scoped, add the zero-generated-token flush regression, and run the frontend compile/tests.

Once that PR lands, I’ll rebase #865 onto the updated main so it can rely on the upstream detokenizer fix.

@RicardoMin
RicardoMin force-pushed the fix/request-stop-token-policy branch from ad9499f to 1428bfd Compare August 20, 2026 15:10
@RicardoMin RicardoMin changed the title fix(qwen3): separate EOS from request stop tokens fix(frontend): unify stop-token semantics across model schedulers Aug 20, 2026
@RicardoMin
RicardoMin force-pushed the fix/request-stop-token-policy branch from 1428bfd to 8fc4da0 Compare August 21, 2026 06:00
Signed-off-by: RicardoMin <17879681016@163.com>
@RicardoMin
RicardoMin force-pushed the fix/request-stop-token-policy branch from 8fc4da0 to b9afd91 Compare August 21, 2026 09:24
@RicardoMin

Copy link
Copy Markdown
Contributor Author

Summary

Building on the earlier vLLM compatibility update, this PR completes PegaInfer's token-level stopping contract.

PegaInfer previously mixed model EOS tokens, request-provided stop_token_ids, and the high-level finish_reason. As a result, the engine could know that a request stopped without reliably preserving which token caused it, and ignore_eos could unintentionally affect explicit stop tokens.

This PR aligns PegaInfer's token-level EOS and explicit stop-token semantics with vLLM and carries a typed stop cause through the shared request lifecycle and all migrated scheduler paths.

Contract

EOS and explicit request stop tokens are independent.

Condition finish_reason Internal cause Wire stop_reason
Model EOS, with EOS handling enabled stop Some(StopCause::Eos(token_id)) null
Explicit stop_token_ids match stop Some(StopCause::Token(token_id)) Actual token ID
Output budget reached length None null

When EOS handling is enabled, EOS classification takes precedence if the same token ID also appears in stop_token_ids. When ignore_eos=true, that EOS classification is disabled, while explicit request stop tokens remain active.

The resulting behavior is:

  • ignore_eos=true disables only model EOS termination.
  • Explicit stop_token_ids remain active when EOS is ignored.
  • The triggering token and its logprob are committed before termination is finalized.
  • The triggering token is emitted exactly once.
  • Completion-token accounting includes the triggering token exactly once.
  • Speculative decoding keeps the prefix through the first terminal token and discards the remaining accepted suffix.
  • Explicit stop tokens are reported through the wire-level stop_reason; model EOS does not produce a wire stop_reason.

Changes

  • Added shared EosPolicy, StopPolicy, and StopCause types.
  • Propagated the policy through request, ledger, step, terminal, and event contracts.
  • Migrated Qwen3, Qwen3.5, DeepSeek-V2-Lite, Kimi-K2, GLM5.2, K3, Gemma4, and the simulator.
  • Updated prefill, decode, speculative, and P/D handoff paths to preserve the terminal token and typed stop cause.
  • Updated both stepped and legacy vLLM bridges to report the real stop cause.
  • Fixed a legacy-bridge race where the trigger token and Finished event could arrive in separate channel bursts.
  • Retained the synthetic sentinel only as a compatibility fallback for legacy producers that still return no typed cause.
  • Reject non-zero min_tokens instead of silently ignoring it, because current scheduler and sampler contracts do not yet implement EOS/stop-token masking.

This change is limited to request lifecycle and host-side termination handling. It does not modify model math, CUDA kernels, attention paths, sampling kernels, or CUDA Graph shapes.

Automated Verification

Command Result
cargo test --release -p pegainfer-frontend --lib 79 passed, 0 failed
cargo test --release -p pegainfer-qwen3 --lib 93 passed, 0 failed
cargo test --release -p pegainfer-deepseek-v2-lite --lib 5 passed, 0 failed
cargo test --release -p pegainfer-sim --tests -- --test-threads=1 22 passed, 0 failed
cargo build --release -p pegainfer-server --bin pegainfer Passed
cargo fmt --all -- --check Passed
git diff --check upstream/main...HEAD Passed
cargo test --release -p pegainfer-k3 --lib scheduler::tests Blocked before Rust test compilation by missing CUDA fabric-memory declarations and TileLang

The frontend suite includes a deterministic split-burst regression test that sends the candidate stop token and terminal event separately. It verifies that:

  • the terminal token is not exposed prematurely;
  • the token is emitted exactly once;
  • its token ID is preserved;
  • its logprob is preserved;
  • the final wire stop_reason contains the actual triggering token ID.

Real Qwen3 HTTP Verification

Environment:

  • Model: Qwen3-0.6B
  • Served model name: qwen3-0.6b
  • Endpoint: http://127.0.0.1:18080/v1/completions
  • Maximum model length: 40960
Request HTTP status finish_reason stop_reason Completion tokens Observation
ignore_eos=true, max_tokens=8 200 length null 8 Generation reached the requested limit with ignore_eos enabled
ignore_eos=true, explicit stop set covering the vocabulary 200 stop 12095 1 Explicit stop remained active and reported the actual triggering token
min_tokens=1 500 N/A N/A N/A Rejected before scheduler submission; the current frontend maps this engine-side rejection to the standard internal-error response

The full-vocabulary stop set is intentionally a deterministic stop-path test, not a generation-quality benchmark.

The min_tokens=1 request is expected to be rejected because the current scheduler contracts do not carry the threshold required for EOS and explicit stop-token masking.

Known Limitations

  • min_tokens masking is not implemented. Supporting it requires adding the minimum-generation threshold to the scheduler contract and every sampler path.
  • The legacy bridge still retains a synthetic-sentinel fallback for non-migrated producers. The fallback is bypassed whenever a real typed StopCause is present.
  • Feature-gated model paths beyond the listed automated suites received static and contract-level review, but full real-GPU end-to-end validation for every model was not available on this test host.

Scope

This PR establishes a typed, request-scoped stop contract for EOS and explicit stop tokens while preserving backward compatibility with legacy producers.

Follow-up work can remove the synthetic sentinel after all remaining legacy producers have migrated to typed StopCause reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants